Skip to content

Conversation

@AcKoucher
Copy link
Contributor

@AcKoucher AcKoucher commented Apr 1, 2025

Context

When running Secure-CI for #6649, some results were considerably different than master which doesn't seem to make sense as there's no functionality changes.

Cause of the Differences

Inside SA, we have a mechanism to store the best valid result. It was created in order to ensure MPL converges if a valid result was generated at any point during annealing. However, this best stored result is only actually used if the final result is invalid. We should use the best valid result even if the final iteration result is a valid one, because we might have generated a good result during the early iterations that ended up being discarded.

I.e., what's happening in the previously cited PR is that the change in the random number generator affected the path SA takes and some good results from early iterations that we were luckily choosing are now being discarded.

Changes

Use the best valid result stored even if the final result is valid.
With these changes, the results should improve compared to master and the results between base/test for #6649 should get more consistent.

Additional

The io_constraints3 test is not using the boundary penalty in SA. That will make the test more concise as the centralization attempt should always work regardless of the shape chosen for the std cell cluster.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 1, 2025

clang-tidy review says "All clean, LGTM! 👍"

@QuantamHD
Copy link
Collaborator

Noice!

@maliberty
Copy link
Member

Are you running a secure CI for this? Hopefully we just see improvements...

@AcKoucher
Copy link
Contributor Author

AcKoucher commented Apr 2, 2025

I ran a Secure-CI and it had some failures. I'm taking a look.

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@AcKoucher
Copy link
Contributor Author

Re-running Secure-CI.

@AcKoucher
Copy link
Contributor Author

uW is failing due to high congestion in GRT post-drt. I'm taking a look.

@AcKoucher
Copy link
Contributor Author

AcKoucher commented May 2, 2025

Re-ran CI with a lower routing layer adjustment for uW (0.15 -> 0.10) and it passed. However, ngt45/swerv_wrapper is failing in detail placement. I'll investigate.

AcKoucher added 3 commits May 5, 2025 14:05
   1) Change test to not consider boundary penalty in
      order to make it more concise.
   2) Adapt regression tests results.

Signed-off-by: Arthur Koucher <[email protected]>
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@AcKoucher
Copy link
Contributor Author

AcKoucher commented May 23, 2025

The IO constraints project was being prioritized so this was on hold.

I updated the branch with master and adapted the tests results. I'm also updating a test's configuration to make it more concise.

Re-running Secure-CI.

@AcKoucher
Copy link
Contributor Author

ngt45/swerv_wrapper is faling during GRT due high congestion. The only difference with master is that the order of the small RAMs changed inside their array. I'm taking a look.

@AcKoucher
Copy link
Contributor Author

#3190 for decreasing routing layer adjustment on ngt45/swerv_wrapper. Running final Secure-CI to double-check everything.

@AcKoucher AcKoucher requested a review from eder-matheus May 26, 2025 19:01
@AcKoucher
Copy link
Contributor Author

@eder-matheus Secure-CI passed.

@eder-matheus eder-matheus merged commit dbe7a4e into The-OpenROAD-Project:master May 27, 2025
11 checks passed
@AcKoucher AcKoucher deleted the mpl-use-best-result branch May 27, 2025 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants